Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid breaking protobuf release for now #4634

Closed
wants to merge 1 commit into from

Conversation

lepistone
Copy link

Protobuf-java was upgraded to 4.28.2 to address #4584.

The vulnerability
CVE-2024-7254 is
fixed in protobuf-java 3.25.5, as initially suggested in #4584.

Protobuf-java saw major breaking changes in 4.26, partially mitigated in
the 27 series. Because it takes time to adopt to these breaking changes,
it is better I think to only address the vulnerability and not jump into
the breaking releases yet.

Specifically, the problem is that now everyone that uses error-prone is
forced to jump to the breaking Protobuf releases today.

This includes all users of the chain of Google BOMs (libraries-bom,
first-party-dependencies, google-cloud-bom and
gapic-generator-java-bom). Those still reference 3.25.5 [1].

This PR fixes the issue. I think error-prone should then be released and
included in gapic-generator-java-pom-parent.

Thank you!

[1] https://github.com/googleapis/sdk-platform-java/blob/main/gapic-generator-java-pom-parent/pom.xml#L34

Protobuf-java was upgraded to 4.28.2 to address google#4584.

The vulnerability
[CVE-2024-7254](GHSA-735f-pc8j-v9w8) is
fixed in protobuf-java 3.25.5, as initially suggested in google#4584.

Protobuf-java saw major breaking changes in 4.26, partially mitigated in
the 27 series. Because it takes time to adopt to these breaking changes,
it is better I think to only address the vulnerability and not jump into
the breaking releases yet.

Specifically, the problem is that now everyone that uses error-prone is
forced to jump to the breaking Protobuf releases today.

This includes all users of the chain of Google BOMs (libraries-bom,
first-party-dependencies, google-cloud-bom and
gapic-generator-java-bom). Those still reference 3.25.5 [1].

This PR fixes the issue. I think error-prone should then be released and
included in gapic-generator-java-pom-parent.

Thank you!

[1] https://github.com/googleapis/sdk-platform-java/blob/main/gapic-generator-java-pom-parent/pom.xml#L34
@tbroyer
Copy link
Contributor

tbroyer commented Oct 23, 2024

Why is this change forcing "everyone that uses error-prone […] to jump to the breaking Protobuf releases"? Error Prone only lives in the processor path, segregated from the compile classpath, so it should be OK to have Protobuf 4.x on the processor path and Protobuf 3.x in the compile classpath.

Or are you talking about Error Prone plugins or annotation processors or other JavaC plugins using Protobuf?

@lepistone
Copy link
Author

Why is this change forcing "everyone that uses error-prone […] to jump to the breaking Protobuf releases"? Error Prone only lives in the processor path, segregated from the compile classpath, so it should be OK to have Protobuf 4.x on the processor path and Protobuf 3.x in the compile classpath.

Or are you talking about Error Prone plugins or annotation processors or other JavaC plugins using Protobuf?

Thanks for your reply! I think you are right in saying that the plugin version should be separate from runtime dependencies.

However, the problem we have in practice is that we have a company-wide BOM, which imports the Google chain of BOMs, plus others that we set ourselves. In that BOM we use the maven-enforcer-plugin with requireUpperBounds to avoid a class of problems with versions.

Currently we have different error-prone artifacts in our BOM, and those now fail the maven-enforcer-plugin check. We haven't checked yet if we could remove from our BOM all artifacts other than the annotations from our BOM and if others are used as runtime dependencies somewhere.

We'll check if we can mitigate our issue by removing error-prone artifacts from our BOM (assuming they are not used).

That said, I still think it's a bit too early for jumping to protobuf 4.28.2 now, for no real gain. Thank you!

@lepistone
Copy link
Author

Update: we indeed have custom error-prone plugins that use error-prone-core in addition to usage of the annotations, and we manage all versions with a company-wide BOM that imports the Google Cloud BOM. So even though the Google Cloud BOM only specifies the annotations, we keep all the error-prone artifacts in sync in our own BOM.
So in order to address the situation we would need to have different versions in our BOM: older releases for -core and other artifacts that depend on protobuf, and the latest version of -annotations as controlled by the Google Cloud BOM. I don't know if using error-prone artifacts of different versions in the same project can lead to problems (new -annotations, old -core).

I would still say that unless error-prone has a concrete need to have protobuf 28 today, this bump creates unnecessary friction to teams (like us) that use the Google Cloud BOM, and also to teams that are going through a painful migration of the breaking changes introduced from version 26 (also like us). Thanks!

copybara-service bot pushed a commit that referenced this pull request Oct 25, 2024
This still fixes GHSA-735f-pc8j-v9w8, but avoids a breaking change to 4.x, see #4634

PiperOrigin-RevId: 689455267
@cushon
Copy link
Collaborator

cushon commented Oct 25, 2024

I'm going to cut an Error Prone release that downgrades the proto dependency back to 3.25.5 to match the Google Cloud BOM, and then upgrade the dependency back to 4.x for future releases.

copybara-service bot pushed a commit that referenced this pull request Oct 25, 2024
This still fixes GHSA-735f-pc8j-v9w8, but avoids a breaking change to 4.x, see #4634

PiperOrigin-RevId: 689792648
@cushon
Copy link
Collaborator

cushon commented Oct 25, 2024

I'm going to cut an Error Prone release that downgrades the proto dependency back to 3.25.5 to match the Google Cloud BOM, and then upgrade the dependency back to 4.x for future releases.

A 2.35.1 release that uses protobuf 3.25.5 is available: https://github.com/google/error-prone/releases/tag/v2.35.1

@cushon cushon closed this Oct 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants